Android: Set proper versionCode and versionName #2398

hoffie · 2022-02-16T23:40:05Z

Short description of changes

Autobuild: Allow overriding checkout's fetch-depth
Android: Set ANDROID_VERSION_{CODE,NAME}
- android/AndroidManifest.xml uses specific macros (%%INSERT_VERSION...). Those macros are replaced by the ANDROID_VERSION_* qmake variables, which were previously unset and therefore defaulted to "1". This made it impossible to update the APK without uninstalling first according to Android: Package update installation fails #1760. This commit populates these variables.
- The user-visible ANDROID_VERSION_NAME is set similar to VERSION for other platforms.
- The Android-internal ANDROID_VERSION_CODE is set to the number of git commits in the current branch. This seems to be the most straightforward solution which guarantees unique, increasing integers within the scope of the master branch. Feature branches will break this logic, so developers may still be required to uninstall/reinstall.
- In order to get the commit count, the autobuild Android logic had to be modified to fetch the repo in full depth instead of only the most-recent commit.
- Tested via apktool d and by checking apktool.yml.
Jamulus.pro: Move ANDROID_ABIS to conditional (The variable was set globally, but is only required for the Android target. Git history does not show a specific reason for placing it globally (a3558aa).)
Jamulus.pro: Use DISTFILES for AndroidManifest.xml
- Official docs suggest using DISTFILES: https://doc.qt.io/qt-5/deployment-android.html
- OTHER_FILES is not documented, while DISTFILES is. https://doc.qt.io/qt-5/qmake-variable-reference.html#distfiles https://stackoverflow.com/questions/38102160/in-qt-when-should-you-use-resources-vs-distfiles-vs-other-files/38102991#38102991

Context: Fixes an issue?

Related #1760 (but doesn't fix, it seems)

Does this change need documentation? What needs to be documented and how?

No.
CHANGELOG: Android: Package version has been fixed to show the actual release version.

Status of this Pull Request

Ready.

What is missing until this pull request can be merged?

Can be merged after 3.8.2.

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want (via apktool)
~~My code follows the style guide~~
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

hoffie · 2022-02-16T23:41:30Z

@NickHyHo Can you confirm that this PR fixes your issue? You can get the APK from the "Checks" listed above.
@ann0see Were you able to reproduce the issue with previous APKs? Can you confirm that the APK from this PR works better?

NickHyHo · 2022-02-17T00:39:03Z

@hoffie First, a Government Health Warning: My coding days are long (30+ years!) gone, and my training/experience on GitHub is zilch, but here goes... Managed to find an APK "3.8.2rc1dev-c50f092 (1)" which I installed ok, but I have a feeling that it made itself at home as a brand new package... so I then found APK "3.8.2rc1dev-26b235b0 (4749)" hot off the press, attempted to install, got "This package is already installed ... {2x package details} ... Do you want to install the update", so I did attempt install, but got "App not installed as package conflicts with an existing package." It's now a bit late here (UK) so I won't try to untangle myself until tomorrow... Since I'm junior at GitHub, please could you point me directly to the APK you'd like me to try, I'll then clear down, install the official Jamulus build, put some settings in (so I know if they're maintained during update!), then try to install your new APK on top... Or another process if you wish!

NickHyHo · 2022-02-17T11:10:32Z

@hoffie Doh! I used wrong device last night (2 similar phones, tired person, any excuse...). This morning I used the correct device, attempted install of 3.8.2rc1dev-26b235b0 (4749) over 3.8.0, but it still ended with "App not installed". I don't know if I used the correct Release Candidate, but my erroneous work last night might suggest that there's still a problem anyway?

pljones · 2022-02-17T17:12:14Z

I don't know if I used the correct Release Candidate, but my erroneous work last night might suggest that there's still a problem anyway?

The assets from the latest run are here (as far as I can tell...):
https://github.com/jamulussoftware/jamulus/actions/runs/1855822646

It looks like that's the one you used.

NickHyHo · 2022-02-17T17:17:19Z

@pljones Thanks yes, that's the one, so I think it's not quite fixed yet...

hoffie · 2022-02-17T22:57:45Z

@NickHyHo Thanks for testing. So this PR doesn't seem to be enough to fix the issue. I still believe it's right and improves things (we were always version 1 before that PR), but there's no hurry in getting it merged. I'm leaving it open for merge after 3.8.2.
I think we should move further debugging to #1760 again.

ann0see · 2022-02-21T06:46:09Z

Unfortunately I don’t have access to the Android device anymore.

softins

Looks good so far, but I'd like to try an installation, and maybe even a build myself.

softins · 2022-02-21T22:25:52Z

.github/workflows/autobuild.yml

@@ -168,6 +169,7 @@ jobs:
        uses:                       actions/checkout@v2
        with:
          submodules:               true
+          fetch-depth:              ${{ matrix.config.checkout_fetch_depth || '1' }}


Does the checkout_fetch_depth: '0' further up evaluate here as false or true?

I haven't found explicit documentation, but the CI confirms that it does what it should: Android uses a full checkout while the other targets (e.g. Linux) only fetch the most recent commit:

Therefore, I assume that '0' as a non-empty string evaluates to true (probably like Python).

Therefore, I assume that '0' as a non-empty string evaluates to true (probably like Python).

And unlike Perl and PHP!

softins · 2022-02-21T22:39:38Z

Jamulus.pro

+    ANDROID_ABIS = armeabi-v7a arm64-v8a x86 x86_64
+    ANDROID_VERSION_NAME = $$VERSION
+    ANDROID_VERSION_CODE = $$system(git log --oneline | wc -l)
+


I couldn't see in the build log where these get used. If they are used silently, then perhaps they could be echoed too?

Yep, they're qmake-internal variables. I've added a message() call to output them now.

Just tried installing jamulus_3.8.2dev-5f108fd_android.apk over 3.8.0 ... result is as before, i.e. version number is reported as being different (see screenshot attached), but install ends with message "App not installed". I suspect this is "as expected" at the moment! BTW, if you want me to check any functionality on any new build let me know - I have several Android devices at my disposal, and can force a new build to install by uninstalling any old one, of course.

Sorry, first time I've posted an image - didn't realise it'd come out that big on screen!

(I've edited your comment to use <img src="...existing url.." width="2002>to reduce the size a bit.

result is as before, i.e. version number is reported as being different (see screenshot attached)
but install ends with message "App not installed". I suspect this is "as expected" at the moment!

Thanks for the screenshot. Yes, seeing the real version number here is definitely an improvement. And yes, it's expected that this PR does not solve all update problems on Android. While I had hoped for this to be the case, the PR doesn't claim to do it :)

BTW, if you want me to check any functionality on any new build let me know - I have several Android devices at my disposal, and can force a new build to install by uninstalling any old one, of course.

Great, thanks! I'll likely ask again as part of #2404.

android/AndroidManifest.xml uses specific macros (%%INSERT_VERSION...). Those macros are replaced by the ANDROID_VERSION_* qmake variables, which were previously unset and therefore defaulted to "1". This made it impossible to update the APK without uninstalling first. This commit populates these variables. The user-visible ANDROID_VERSION_NAME is set similar to VERSION for other platforms. The Android-internal ANDROID_VERSION_CODE is set to the number of git commits in the current branch. This seems to be the most straightforward solution which guarantees unique, increasing integers within the scope of the master branch. Feature branches will break this logic, so developers may still be required to uninstall/reinstall. In order to get the commit count, the autobuild Android logic had to be modified to fetch the repo in full depth instead of only the most-recent commit. Tested via `apktool d` and by checking `apktool.yml`. Related: jamulussoftware#1760

The variable was set globally, but is only required for the Android target. Git history does not show a specific reason for placing it globally (a3558aa).

Official docs suggest using DISTFILES: https://doc.qt.io/qt-5/deployment-android.html OTHER_FILES is undocumented, while DISTFILES is. https://doc.qt.io/qt-5/qmake-variable-reference.html#distfiles https://stackoverflow.com/questions/38102160/in-qt-when-should-you-use-resources-vs-distfiles-vs-other-files/38102991#38102991

hoffie · 2022-02-26T16:11:19Z

@ann0see @softins IMO we could merge this soon. Android is still experimental and both apktool and @NickHyHo's tests/screenshot show that the version handling seems to have been improved with this PR. I think the chance that this PR makes anything worse is rather low. We should have a close look at the Autobuild fetch depth change, but there are at least logs which show that it works as expected.

ann0see

I can't test it at the moment, but I think it's ok.

softins

The changes look ok, and I was able to install the apk on my phone. Settings:Apps reports the correct version number, and I can run the app. Some issues in the app itself to address, but they are not related to this PR, so can be investigated separately.

ann0see · 2022-07-25T17:28:52Z

@pljones and another fix (prefixed with Android: now)

hoffie added this to the Release 3.9.0 milestone Feb 16, 2022

hoffie force-pushed the autobuild/set-android-version branch from 4920476 to 967b57b Compare February 17, 2022 22:59

hoffie mentioned this pull request Feb 19, 2022

Android: Package update installation fails #1760

Open

hoffie force-pushed the autobuild/set-android-version branch from 967b57b to e61419b Compare February 20, 2022 22:58

hoffie requested review from ann0see and softins February 20, 2022 23:02

ann0see removed their request for review February 21, 2022 06:46

softins reviewed Feb 21, 2022

View reviewed changes

hoffie added 4 commits February 22, 2022 00:10

Autobuild: Allow overriding checkout's fetch-depth

cc533bd

Jamulus.pro: Move ANDROID_ABIS to conditional

b49dec4

The variable was set globally, but is only required for the Android target. Git history does not show a specific reason for placing it globally (a3558aa).

hoffie force-pushed the autobuild/set-android-version branch from e61419b to 079a52a Compare February 21, 2022 23:10

ann0see approved these changes Feb 26, 2022

View reviewed changes

softins approved these changes Feb 26, 2022

View reviewed changes

hoffie merged commit 6117258 into jamulussoftware:master Feb 26, 2022

hoffie deleted the autobuild/set-android-version branch February 26, 2022 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android: Set proper versionCode and versionName #2398

Android: Set proper versionCode and versionName #2398

hoffie commented Feb 16, 2022 •

edited by ann0see

Loading

hoffie commented Feb 16, 2022

NickHyHo commented Feb 17, 2022

NickHyHo commented Feb 17, 2022

pljones commented Feb 17, 2022 •

edited

Loading

NickHyHo commented Feb 17, 2022

hoffie commented Feb 17, 2022

ann0see commented Feb 21, 2022

softins left a comment

softins Feb 21, 2022

hoffie Feb 21, 2022

softins Feb 21, 2022

softins Feb 21, 2022

hoffie Feb 21, 2022 •

edited

Loading

NickHyHo Feb 22, 2022 •

edited by hoffie

Loading

NickHyHo Feb 22, 2022

hoffie Feb 26, 2022

hoffie commented Feb 26, 2022

ann0see left a comment

softins left a comment

ann0see commented Jul 25, 2022

Android: Set proper versionCode and versionName #2398

Android: Set proper versionCode and versionName #2398

Conversation

hoffie commented Feb 16, 2022 • edited by ann0see Loading

Checklist

hoffie commented Feb 16, 2022

NickHyHo commented Feb 17, 2022

NickHyHo commented Feb 17, 2022

pljones commented Feb 17, 2022 • edited Loading

NickHyHo commented Feb 17, 2022

hoffie commented Feb 17, 2022

ann0see commented Feb 21, 2022

softins left a comment

Choose a reason for hiding this comment

softins Feb 21, 2022

Choose a reason for hiding this comment

hoffie Feb 21, 2022

Choose a reason for hiding this comment

softins Feb 21, 2022

Choose a reason for hiding this comment

softins Feb 21, 2022

Choose a reason for hiding this comment

hoffie Feb 21, 2022 • edited Loading

Choose a reason for hiding this comment

NickHyHo Feb 22, 2022 • edited by hoffie Loading

Choose a reason for hiding this comment

NickHyHo Feb 22, 2022

Choose a reason for hiding this comment

hoffie Feb 26, 2022

Choose a reason for hiding this comment

hoffie commented Feb 26, 2022

ann0see left a comment

Choose a reason for hiding this comment

softins left a comment

Choose a reason for hiding this comment

ann0see commented Jul 25, 2022

hoffie commented Feb 16, 2022 •

edited by ann0see

Loading

pljones commented Feb 17, 2022 •

edited

Loading

hoffie Feb 21, 2022 •

edited

Loading

NickHyHo Feb 22, 2022 •

edited by hoffie

Loading